-
Notifications
You must be signed in to change notification settings - Fork 14k
rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution #149043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution #149043
Conversation
|
rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing |
This comment was marked as outdated.
This comment was marked as outdated.
c7f4e67 to
2e1b954
Compare
|
Makes sense to me! Ping me once PR is ready for final review. :) |
This is useful for allowing writing run-make tests that test rustdoc-json.
2e1b954 to
4d00c1a
Compare
e4cdd0c to
457ed4e
Compare
|
The run-make-support library was changed cc @jieyouxu These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
|
@GuillaumeGomez ready for review. |
|
Looks all good to me, thanks! r=me once CI pass |
|
@bors r=GuillaumeGomez rollup |
…-out-of-spite-does-it-matter, r=GuillaumeGomez rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution Historically, it's not been possible to robustly resolve a cross-crate item in rustdoc-json. If you had a `Id` that wasn't in `Crate::index` (because it was defined in a different crate), you could only look it up it `Crate::paths`. But there, you don't get the full information, only an `ItemSummary`. This tells you the `path` and the `crate_id`. But knowing the `crate_id` isn't enough to be able to build/find the rustdoc-json output with this item. It's only use is to get a `ExternalCrate` (via `Crate::external_crates`). But that only tells you the `name` (as a string). This isn't enough to uniquely identify a crate, as there could be multiple versions/features [^1] [^2]. This was originally proposed to be solved via `@LukeMathWalker's` `--orchestrator-id` proposal (rust-lang/compiler-team#635). But that requires invasive changes to cargo/rustc. This PR instead implements `@Urgau's` proposal to re-use the path to a crate's rmeta/rlib as a unique identifer. Callers can use that to determine which package it corresponds to in the language of the build-system above rustc. E.g. for cargo, `cargo rustdoc --message-format=json --output-format=json -Zunstable-options`). (Once you've found the right external crate's rustdoc-json output, you still need to resolve the path->id in that crate. But that's """just""" a matter of walking the module tree. We should probably still make that nicer (by, for example, allowing sharing `Id`s between rustdoc-json document), but that's a future concern) For some notes from RustWeek 2025, where this was designed, see https://hackmd.io/0jkdguobTnW7nXoGKAxfEQ CC `@obi1kenobi` (who wants this for cargo-semver-checks [^3]), `@epage` (who's conversations on what and wasn't possible with cargo informed taking this approach to solve this problem) r? `@GuillaumeGomez` ## TODO: - [x] Docs: [Done](https://github.com/rust-lang/rust/compare/e4cdd0c24a994fed354081b5f907680a11f2ddc5..457ed4edb184997d5d6f879c6a220bc4d69ff6fd) - [x] Tests: [Done](https://github.com/rust-lang/rust/compare/2e1b954dc52bf7e5a6e9311394df760db37d383f..4d00c1a7ee5e03d1e78801cc01a85dac08ab603b) [^1]: rust-lang/compiler-team#635 (comment) § Problem [^2]: https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/Identifying.20external.20crates.20in.20Rustdoc.20JSON/with/352701211 [^3]: obi1kenobi/cargo-semver-checks#638
|
Lemme try something. @bors try @rust-timer queue profiles=DocJson |
|
Error occurred while parsing comment: Cannot parse profiles: Invalid profile: DocJson. Valid values are: check, debug, opt, doc, doc-json, clippy |
…te-does-it-matter, r=<try> rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution
This comment has been minimized.
This comment has been minimized.
|
@rust-timer queue profiles=doc-json |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (559b9c4): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (primary 0.3%, secondary -0.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.681s -> 472.815s (0.24%) |
|
Looks like this didn't have any perf. effect. |
…esolution Historically, it's not been possible to robustly resolve a cross-crate item in rustdoc-json. If you had a `Id` that wasn't in `Crate::index` (because it was defined in a different crate), you could only look it up it `Crate::paths`. But there, you don't get the full information, only an `ItemSummary`. This tells you the `path` and the `crate_id`. But knowing the `crate_id` isn't enough to be able to build/find the rustdoc-json output with this item. It's only use is to get a `ExternalCrate` (via `Crate::external_crates`). But that only tells you the `name` (as a string). This isn't enough to uniquely identify a crate, as there could be multiple versions/features [1] [2]. This was originally proposed to be solved via LukeMathWalker's `--orchestrator-id` proposal (https://www.github.com/rust-lang/compiler-team/issues/635). But that requires invasive changes to cargo/rustc. This PR instead implements Urgau's proposal to re-use the path to a crate's rmeta/rlib as a unique identifer. Callers can use that to determine which package it corresponds to in the language of the build-system above rustc. E.g. for cargo, `cargo rustdoc --message-format=json --output-format=json -Zunstable-options`). (Once you've found the right external crate's rustdoc-json output, you still need to resolve the path->id in that crate. But that's """just""" a matter of walking the module tree. We should probably still make that nicer (by, for example, allowing sharing `Id`s between rustdoc-json document), but that's a future concern) For some notes from RustWeek 2025, where this was designed, see https://hackmd.io/0jkdguobTnW7nXoGKAxfEQ [1]: https://www.github.com/rust-lang/compiler-team/issues/635#issue-1714254865 § Problem [2]: https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/Identifying.20external.20crates.20in.20Rustdoc.20JSON/with/352701211
457ed4e to
a47dff8
Compare
|
@bors try jobs=dist-various1 |
This comment has been minimized.
This comment has been minimized.
…te-does-it-matter, r=<try> rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution try-job: dist-various1
|
💔 Test for a107de6 failed: CI. Failed job:
|
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
@bors try jobs=dist-various-1 |
…te-does-it-matter, r=<try> rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution try-job: dist-various-1
This comment has been minimized.
This comment has been minimized.
|
@bors r=GuillaumeGomez rollup |
…-out-of-spite-does-it-matter, r=GuillaumeGomez rustdoc-json: add rlib path to ExternalCrate to enable robust crate resolution Historically, it's not been possible to robustly resolve a cross-crate item in rustdoc-json. If you had a `Id` that wasn't in `Crate::index` (because it was defined in a different crate), you could only look it up it `Crate::paths`. But there, you don't get the full information, only an `ItemSummary`. This tells you the `path` and the `crate_id`. But knowing the `crate_id` isn't enough to be able to build/find the rustdoc-json output with this item. It's only use is to get a `ExternalCrate` (via `Crate::external_crates`). But that only tells you the `name` (as a string). This isn't enough to uniquely identify a crate, as there could be multiple versions/features [^1] [^2]. This was originally proposed to be solved via `@LukeMathWalker's` `--orchestrator-id` proposal (rust-lang/compiler-team#635). But that requires invasive changes to cargo/rustc. This PR instead implements `@Urgau's` proposal to re-use the path to a crate's rmeta/rlib as a unique identifer. Callers can use that to determine which package it corresponds to in the language of the build-system above rustc. E.g. for cargo, `cargo rustdoc --message-format=json --output-format=json -Zunstable-options`). (Once you've found the right external crate's rustdoc-json output, you still need to resolve the path->id in that crate. But that's """just""" a matter of walking the module tree. We should probably still make that nicer (by, for example, allowing sharing `Id`s between rustdoc-json document), but that's a future concern) For some notes from RustWeek 2025, where this was designed, see https://hackmd.io/0jkdguobTnW7nXoGKAxfEQ CC `@obi1kenobi` (who wants this for cargo-semver-checks [^3]), `@epage` (who's conversations on what and wasn't possible with cargo informed taking this approach to solve this problem) r? `@GuillaumeGomez` ## TODO: - [x] Docs: [Done](https://github.com/rust-lang/rust/compare/e4cdd0c24a994fed354081b5f907680a11f2ddc5..457ed4edb184997d5d6f879c6a220bc4d69ff6fd) - [x] Tests: [Done](https://github.com/rust-lang/rust/compare/2e1b954dc52bf7e5a6e9311394df760db37d383f..4d00c1a7ee5e03d1e78801cc01a85dac08ab603b) [^1]: rust-lang/compiler-team#635 (comment) § Problem [^2]: https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/Identifying.20external.20crates.20in.20Rustdoc.20JSON/with/352701211 [^3]: obi1kenobi/cargo-semver-checks#638
Historically, it's not been possible to robustly resolve a cross-crate item in rustdoc-json. If you had a
Idthat wasn't inCrate::index(because it was defined in a different crate), you could only look it up itCrate::paths. But there, you don't get the full information, only anItemSummary. This tells you thepathand thecrate_id.But knowing the
crate_idisn't enough to be able to build/find the rustdoc-json output with this item. It's only use is to get aExternalCrate(viaCrate::external_crates). But that only tells you thename(as a string). This isn't enough to uniquely identify a crate, as there could be multiple versions/features 1 2.This was originally proposed to be solved via @LukeMathWalker's
--orchestrator-idproposal (rust-lang/compiler-team#635). But that requires invasive changes to cargo/rustc. This PR instead implements @Urgau's proposal to re-use the path to a crate's rmeta/rlib as a unique identifer. Callers can use that to determine which package it corresponds to in the language of the build-system above rustc. E.g. for cargo,cargo rustdoc --message-format=json --output-format=json -Zunstable-options).(Once you've found the right external crate's rustdoc-json output, you still need to resolve the path->id in that crate. But that's """just""" a matter of walking the module tree. We should probably still make that nicer (by, for example, allowing sharing
Ids between rustdoc-json document), but that's a future concern)For some notes from RustWeek 2025, where this was designed, see https://hackmd.io/0jkdguobTnW7nXoGKAxfEQ
CC @obi1kenobi (who wants this for cargo-semver-checks 3), @epage (who's conversations on what and wasn't possible with cargo informed taking this approach to solve this problem)
r? @GuillaumeGomez
TODO:
Footnotes
https://github.com/rust-lang/compiler-team/issues/635#issue-1714254865 § Problem ↩
https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/Identifying.20external.20crates.20in.20Rustdoc.20JSON/with/352701211 ↩
https://github.com/obi1kenobi/cargo-semver-checks/issues/638 ↩